fix(storev2): hold read lock in GetCommitKVStore during concurrent map access#3517
fix(storev2): hold read lock in GetCommitKVStore during concurrent map access#3517amir-deris wants to merge 4 commits into
Conversation
PR SummaryMedium Risk Overview Adds Reviewed by Cursor Bugbot for commit fd1b008. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3517 +/- ##
==========================================
- Coverage 59.05% 58.20% -0.86%
==========================================
Files 2205 2132 -73
Lines 182317 173954 -8363
==========================================
- Hits 107672 101252 -6420
+ Misses 64945 63681 -1264
+ Partials 9700 9021 -679
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
Could we tighten the test? Something like: func TestGetCommitKVStore_ReaderRespectsWriteLock(t *testing.T) {
store := &Store{
storeKeys: map[string]types.StoreKey{},
ckvStores: map[types.StoreKey]types.CommitKVStore{},
}
key := types.NewKVStoreKey("bank")
store.storeKeys[key.Name()] = key
store.ckvStores[key] = mem.NewStore()
store.mtx.Lock()
readDone := make(chan types.CommitKVStore, 1)
go func() {
readDone <- store.GetCommitKVStore(key)
}()
select {
case <-readDone:
t.Fatal("GetCommitKVStore returned while write lock held — RLock missing")
case <-time.After(50 * time.Millisecond):
}
newVal := mem.NewStore()
store.ckvStores = map[types.StoreKey]types.CommitKVStore{key: newVal}
store.mtx.Unlock()
require.Same(t, newVal, <-readDone)
}This deterministically verifies the |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit e5a3ced. Configure here.
|
@codex review this pr |
|
To use Codex here, create a Codex account and connect to github. |

Summary
RLock/RUnlocktoGetCommitKVStoreinrootmulti/store.goso reads of theckvStoresmap are properly synchronized against concurrent writes.TestGetCommitKVStoreNoDataRaceto exercise the concurrent read/write path; run withgo test -race ./sei-cosmos/storev2/rootmulti/....Test plan
go test -race ./sei-cosmos/storev2/rootmulti/...passes with no data race reports